ROSAENG-1331: Add VpcEndpointTemplate MC e2e tests - #463
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@dustman9000: This pull request references ROSAENG-1331 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dustman9000 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughChangesVpcEndpointTemplate end-to-end test
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 12 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/e2e/vpcendpointtemplate_mc_test.go (2)
31-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a stable
Contextnode for the lifecycle scenario.The two
Itnodes are direct children ofDescribe. Wrap the VpcEndpoint lifecycle tests in a stableContext, such asContext("when a HostedControlPlane uses the private-hcp template", ...).As per coding guidelines,
**/*_test.gomust “use Ginkgo/Gomega with Describe, Context, and It for BDD-style tests.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/vpcendpointtemplate_mc_test.go` at line 31, Wrap the lifecycle test cases inside the top-level Describe block in a stable Context describing the private-HCP template scenario, such as “when a HostedControlPlane uses the private-hcp template.” Keep both existing It nodes within that Context and preserve their test behavior.Source: Coding guidelines
62-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd operation-specific assertion messages.
These assertions do not identify the failed Kubernetes operation. Add messages for Namespace creation, HostedControlPlane retrieval, and HostedControlPlane deletion.
As per coding guidelines, “assertions should include meaningful failure messages.”
Also applies to: 102-104
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/vpcendpointtemplate_mc_test.go` at line 62, Add operation-specific failure messages to the assertions in the namespace creation and HostedControlPlane retrieval/deletion flows, including the Expect call around c.Create(ctx, ns) and the related assertions near the referenced lines. Identify each Kubernetes operation clearly as Namespace creation, HostedControlPlane retrieval, or HostedControlPlane deletion while preserving the existing assertion conditions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/vpcendpointtemplate_mc_test.go`:
- Around line 63-72: The cleanup registered around the HostedControlPlane and
namespace in the first It must not run before the second ordered spec verifies
VpcEndpoint deletion. Move both cleanup operations to AfterAll (or combine the
checks into one It) and assert or otherwise handle errors returned by c.Delete
instead of discarding them.
---
Nitpick comments:
In `@test/e2e/vpcendpointtemplate_mc_test.go`:
- Line 31: Wrap the lifecycle test cases inside the top-level Describe block in
a stable Context describing the private-HCP template scenario, such as “when a
HostedControlPlane uses the private-hcp template.” Keep both existing It nodes
within that Context and preserve their test behavior.
- Line 62: Add operation-specific failure messages to the assertions in the
namespace creation and HostedControlPlane retrieval/deletion flows, including
the Expect call around c.Create(ctx, ns) and the related assertions near the
referenced lines. Identify each Kubernetes operation clearly as Namespace
creation, HostedControlPlane retrieval, or HostedControlPlane deletion while
preserving the existing assertion conditions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e106477b-ec09-4327-8a26-07987c82c5bb
📒 Files selected for processing (1)
test/e2e/vpcendpointtemplate_mc_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #463 +/- ##
=======================================
Coverage 45.04% 45.04%
=======================================
Files 32 32
Lines 2360 2360
=======================================
Hits 1063 1063
Misses 1175 1175
Partials 122 122 🚀 New features to boost your workflow:
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/lgtm AI-generated. Review for accuracy. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
New changes are detected. LGTM label has been removed. |
|
/retest ci/prow/validate |
|
/retest |
Test the VpcEndpointTemplate controller on real Management Clusters. Creates a fake HCP in a test namespace, verifies AVO creates a VpcEndpoint from the private-hcp template, then verifies cleanup when the HCP is deleted. Skips on non-MC clusters where the VpcEndpointTemplate CR does not exist.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/retest ci/prow/validate |
|
/retest |
|
@dustman9000: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Summary
This enables the rosa-hcp-mc-e2e Prow job in openshift/release#82850 to actually test AVO's MC-specific controller behavior rather than just CEL validation.
Jira: https://redhat.atlassian.net/browse/ROSAENG-1331
Test plan
Summary by CodeRabbit